feat(sync,cluster-version): KUBECONFIG env var support and informational output#69
Open
onuryilmaz wants to merge 5 commits into
Open
feat(sync,cluster-version): KUBECONFIG env var support and informational output#69onuryilmaz wants to merge 5 commits into
onuryilmaz wants to merge 5 commits into
Conversation
…nal output When no explicit kubeconfig flag or CLOUDCTL_* env var is set, fall back to the standard KUBECONFIG environment variable (multi-file merge, same as kubectl) instead of always using ~/.kube/config. A new resolveKubeconfig() helper in root.go detects the default value and returns "" when KUBECONFIG is set, letting client-go's standard loading rules handle multi-file merging. configWithContext() is updated to use NewDefaultClientConfigLoadingRules() when path is "". Both sync and cluster-version now print an informational summary line to stdout (and slog.Info to stderr) before the spinner/query showing which kubeconfig files, context, and namespace are active. Also adds the missing `update` command to the README Commands section and documents the --dry-run flag for sync. Signed-off-by: onuryilmaz <onur.yilmaz@sap.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances cloudctl’s kubeconfig handling by deferring to client-go’s default loading rules (including multi-file $KUBECONFIG semantics) when users have not explicitly provided kubeconfig paths, and adds user-facing informational output describing the effective kubeconfig/context in use.
Changes:
- Added
resolveKubeconfig(...)anddisplayKubeconfig(...)helpers and updatedconfigWithContext(...)to support empty kubeconfig paths (default loading rules /$KUBECONFIG). - Updated
syncandcluster-versionto use the new resolution behavior and print an informational summary line before contacting the API server. - Updated README documentation and added unit tests asserting flag defaults and kubeconfig resolution behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents $KUBECONFIG fallback semantics and adds missing update section + --dry-run mention. |
| cmd/sync.go | Uses kubeconfig resolution + default loading rules; adds informational output; adjusts load/write behavior under multi-file $KUBECONFIG. |
| cmd/sync_test.go | Adds test asserting kubeconfig flag defaults match clientcmd.RecommendedHomeFile. |
| cmd/root.go | Introduces kubeconfig resolution/display helpers and updates configWithContext to support default loading rules. |
| cmd/root_test.go | Adds unit tests for resolveKubeconfig behavior. |
| cmd/cluster-version.go | Uses kubeconfig resolution + default loading rules; adds informational output before querying the server. |
| cmd/cluster-version_test.go | Adds test asserting --kubeconfig default matches clientcmd.RecommendedHomeFile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- resolveKubeconfig: use viper.IsSet instead of value equality so that explicitly setting --kubeconfig to the default path is correctly treated as an explicit value and does not defer to KUBECONFIG - Remove fmt.Fprintln info line from stdout in sync and cluster-version; slog.Info (stderr) is sufficient and avoids corrupting --output json/yaml pipelines; also fixes the errcheck lint violations - Fail fast with a clear error when KUBECONFIG is set but yields no usable write target (e.g. leading path-list separator) Signed-off-by: onuryilmaz <onur.yilmaz@sap.com>
- cluster-version: add "(unknown)" fallback when effectiveContext is still empty after kubeconfig load (e.g. no current-context set) - root: fix displayKubeconfig to render ~/.kube/config instead of "$KUBECONFIG ()" when path is "" and KUBECONFIG is also unset - sync_test, cluster-version_test: update stale comments that referred to value-equality detection; now correctly describe viper.IsSet usage Signed-off-by: onuryilmaz <onur.yilmaz@sap.com>
- root: broaden resolveKubeconfig doc comment to include config-file as a valid explicit-set source (viper.IsSet covers all three) - cluster-version: restore kubeconfig source in error message so failures include which file/KUBECONFIG was in use - sync, cluster-version: reject explicitly-set empty kubeconfig values with a clear error instead of silently deferring to client-go defaults - sync_test: add table-driven unit tests for writeTarget KUBECONFIG selection logic (multi-file, leading separator, empty env var) - README: change "prints a summary line" to "logs a summary to stderr" for both sync and cluster-version sections Signed-off-by: onuryilmaz <onur.yilmaz@sap.com>
Signed-off-by: onuryilmaz <onur.yilmaz@sap.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
How it works
A new `resolveKubeconfig(viperKey, flagValue)` helper in `cmd/root.go` uses `viper.IsSet(viperKey)` to detect whether the flag/`CLOUDCTL_` env var was explicitly set. When it was not set and `KUBECONFIG` is present, it returns `""` — signalling that `configWithContext` should use `clientcmd.NewDefaultClientConfigLoadingRules()` instead of an explicit path. Explicitly provided values (flag, `CLOUDCTL_` env var, or config file) are always passed through unchanged.
Test plan